fix(session): inject directory into session API calls for worktree support#1382
fix(session): inject directory into session API calls for worktree support#1382gburch wants to merge 8 commits intocode-yeongyu:devfrom
Conversation
…pport When using OpenCode with git worktrees, session API calls fail with NotFoundError because the server can't resolve the correct project directory. The server falls back to process.cwd() which resolves to 'global' instead of the actual worktree's project ID. This fix wraps ctx.client.session with a Proxy that automatically injects query.directory into every session method call, ensuring session operations are correctly scoped to the worktree's project. Fixes session.get, session.messages, session.prompt, and other session APIs when running from git worktrees or alternate workspaces.
|
All contributors have signed the CLA. Thank you! ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
I don't know why this is being ignored, it causes desktop and web to not work with worktrees which is newer functionality that is pretty core. Anyone using them will have to dig DEEP to resolve this issue and the entire system breaks if they don't. Stop adding features and merge this so this add-on works with worktrees. How are you working without them? @code-yeongyu |
code-yeongyu
left a comment
code-yeongyu
left a comment
There was a problem hiding this comment.
PR #1382 Code Review: Worktree Support Fix
Thank you for identifying and fixing this git worktree issue, @gburch! This is a real pain point that affects users working with worktrees. However, I have several concerns about the implementation approach that I'd like to discuss before merging.
🔴 Critical Issues
1. Proxy Blindly Injects Directory to ALL Session Methods
The current implementation wraps ALL session methods with directory injection:
function wrapSession<T extends object>(session: T, directory: string): T {
const handler: ProxyHandler<T> = {
get(target, prop) {
const value = target[prop as keyof T];
if (typeof value !== "function") return value;
return (args?: DirectoryQuery) => {
const next = withDirectoryArgs(args, directory); // ← Always injects
const fn = value as (input: DirectoryQuery) => unknown;
return fn.call(target, next);
};
},
};
return new Proxy(session, handler);
}Problem: Not all session methods accept or need query: { directory }:
| Method | Uses Directory? | Notes |
|---|---|---|
session.get() |
✅ Yes | Needs directory |
session.messages() |
✅ Yes | Needs directory |
session.prompt() |
✅ Yes | Needs directory |
session.summarize() |
✅ Yes | Needs directory |
session.create() |
❓ Unclear | Creates NEW session - directory in body? |
session.abort() |
❌ Likely No | Only needs session ID to abort |
session.status() |
❌ No | Global status, called with { path: undefined } |
session.todo() |
❓ Unclear | Currently no directory in codebase |
session.children() |
❓ Unclear | Needs investigation |
Risk: Injecting query: { directory } to methods that don't expect it may cause errors or undefined behavior with future OpenCode SDK versions.
Recommendation: Create an allowlist of methods that actually need directory injection:
const METHODS_NEEDING_DIRECTORY = new Set(['get', 'messages', 'prompt', 'summarize']);
function wrapSession<T extends object>(session: T, directory: string): T {
return new Proxy(session, {
get(target, prop) {
const value = target[prop as keyof T];
if (typeof value !== "function") return value;
if (!METHODS_NEEDING_DIRECTORY.has(String(prop))) return value;
return (args?: DirectoryQuery) => {
const next = withDirectoryArgs(args, directory);
return (value as Function).call(target, next);
};
},
});
}2. Directly Mutates Shared ctx.client Object
function injectDirectoryClient<T extends { session?: object }>(
client: T,
directory: string,
): T {
if (!client.session) return client;
const wrapped = wrapSession(client.session, directory);
client.session = wrapped as T["session"]; // ← Direct mutation!
return client;
}Problem: ctx.client is a shared object provided by OpenCode to all plugins. Mutating it directly:
- May conflict with other plugins
- Makes behavior unpredictable
- Violates immutability principles
Recommendation: Store the wrapped session separately or use a different injection pattern (similar to how injectServerAuthIntoClient works with _client.setConfig()).
3. No Test Coverage
This PR adds 254 lines of new code (90 lines of actual logic + formatting) with zero test coverage.
Minimum tests needed:
withDirectoryArgs()- test merging behavior, edge caseswrapSession()- test proxy interception works correctlyinjectDirectoryClient()- test client mutation- Integration test with mock session API
🟡 Moderate Concerns
4. Formatting Changes Mixed with Logic Changes
The diff shows +254/-164 lines, but most changes are just Prettier/formatting:
- Trailing commas added
- Line breaks changed
- Indentation adjusted
This makes it very hard to review the actual semantic changes. Consider:
- Separating formatting changes into a separate commit
- Or applying project formatter before making logic changes
5. Edge Case: Conflicting Directory Values
function withDirectoryArgs(args: DirectoryQuery | undefined, directory: string) {
// ...
if (query && "directory" in query) return args; // ← Preserves existing
return { ...args, query: { ...query, directory } };
}Question: What if the existing query.directory is DIFFERENT from ctx.directory? Should we:
- Keep the existing value (current behavior)?
- Override with
ctx.directory? - Log a warning?
This edge case needs explicit documentation.
🟢 Positive Aspects
-
Real Problem Identified: Worktree support is genuinely broken - users in worktrees get
NotFoundErrorfor sessions. -
Root Cause Understood: Correctly identified that
process.cwd()resolves toglobalinstead of the worktree's project ID. -
Centralized Solution: Rather than adding
query: { directory }in 22+ places (which already exists in the codebase!), a central injection point is cleaner.
📋 Suggested Actions
- Create allowlist for methods that need directory injection
- Add unit tests for the new utility functions
- Consider immutable approach instead of mutating
ctx.client - Verify against OpenCode SDK - check which methods actually accept
query.directory - Separate formatting commit from logic changes for cleaner review
Alternative Approaches to Consider
Option A: Typed Helper Functions
export const sessionWithDir = {
get: (client, id, dir) => client.session.get({ path: { id }, query: { directory: dir } }),
messages: (client, id, dir) => client.session.messages({ path: { id }, query: { directory: dir } }),
// ... explicit, type-safe
};Option B: Higher-Order Wrapper (Immutable)
function createDirectoryBoundSession(session, directory) {
return {
get: (args) => session.get({ ...args, query: { ...args.query, directory } }),
messages: (args) => session.messages({ ...args, query: { ...args.query, directory } }),
// Explicitly list only methods that need it
abort: session.abort.bind(session), // Passthrough - no directory needed
// ...
};
}I appreciate the effort to fix this real issue! Let's work together to make the implementation robust. Happy to discuss any of these points.
Additional Code-Specific Concerns1.
|
|
Thanks for the detailed review. I agree with the core concerns and I’m going to revise this implementation before merge. What I agree with and will change
Clarification on “all session methods”I verified current OpenCode SDK/server contracts, and That said, I still agree explicitness is better for maintainability. I’ll change this to an explicit method allowlist for clarity and future safety. Directory conflict semanticsCurrent behavior preserves caller-provided I’ll push an update reflecting the above so we can re-review with a tighter, safer patch. |
Create a directory-bound client wrapper with an explicit allowlist and add tests for merge/passthrough behavior.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 26785066 | Triggered | Bearer Token | 287e463 | src/shared/opencode-server-auth.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
2 participants
Problem
When using OpenCode with git worktrees (or any workspace where
process.cwd()differs from the project directory), session API calls fail withNotFoundError:The session file exists under the correct project directory, but the server looks in
global/because it doesn't receive the directory context.Root Cause
OpenCode's session APIs accept an optional
query: { directory }parameter to resolve which project's session storage to use. Without it, the server falls back toprocess.cwd(), which resolves to project IDglobalinstead of the actual worktree's project ID.The plugin has access to the correct directory via
ctx.directory, but this wasn't being passed to session API calls.Solution
Wrap
ctx.client.sessionwith a Proxy at plugin initialization that automatically injectsquery: { directory: ctx.directory }into every session method call. This ensures all session operations (.get(),.messages(),.prompt(), etc.) are correctly scoped to the workspace's project.Changes
withDirectoryArgs()helper to merge directory into query parameterswrapSession()to create a Proxy that intercepts session method callsinjectDirectoryClient()to apply the wrapper at startupinjectDirectoryClient(ctx.client, ctx.directory)during plugin initializationTesting
Verified that
delegate_taskand other session-dependent tools now work correctly when running OpenCode from a git worktree.Summary by cubic
Fixes session API failures in git worktrees by binding the workspace directory to the client’s session methods. Ensures session reads/writes target the correct project and removes NotFoundError.
Written for commit c717c40. Summary will update on new commits.